feat(web-shell): surface worktree isolation in the new-session empty state - #7365

Thanks for the PR!

Template looks good ✓

Problem: the worktree isolation entry point was buried in the sidebar git-pill dropdown — a real discoverability issue with an existing feature, not a theoretical concern. Before/after screenshots clearly show the old vs new placement.

Direction: aligned. Surfacing an existing feature at the moment of session creation is a straightforward UX improvement. CHANGELOG shows worktree isolation is a well-established feature area with ongoing fixes upstream, so improving its discoverability is relevant.

Size: not applicable — all changes are in packages/web-shell/client/, no core module paths touched.

Approach: the scope feels right. Reusing the existing pendingWorktreeRef / worktreePending state machine means no SDK or daemon changes — the toggle just sets a pending intent. The sidebar simplification (dropdown → direct button) follows naturally from removing the worktree menu item. One observation: the CSS is fairly elaborate (entrance animations, hover glow, micro-interactions) — not a blocker, just noting it's more visual polish than the minimal path would require.

Moving on to code review. 🔍

— Qwen Code · qwen3.7-max

_{Reviewed at e19f022f75f58904c14e24b856f45f412da294cf · re-run with @qwen-code /triage}

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 722622f. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

_{Full-resolution recordings (.webm) are attached to the workflow run.}

— Qwen Code · web-shell visuals

Code Review

Independent proposal: given "worktree entry point is buried in a sidebar dropdown", I would add a toggle in the chat empty state that sets the existing pendingWorktreeRef intent, gate it on trusted + git repo (same as the sidebar), add a cancel button to the pending badge, and simplify the now-single-item dropdown to a direct button.

Comparison: the PR does exactly this. The implementation reuses the existing pending-worktree state machine (pendingWorktreeRef / worktreePending) with no SDK or daemon changes — the toggle is purely a UI intent setter. The sidebar simplification follows naturally.

No critical blockers found. Specific observations:

• App.tsx — worktreeToggleEligible is a plain expression (not memoized), which is fine for a small array find + property check. The handleEnableWorktree / handleCancelWorktree callbacks correctly use empty deps (only refs and state setters). The welcomeHeader memo includes all new dependencies.
• WorkspaceSection.tsx — the DropdownMenu → <button> swap is clean. The onNewWorktreeSession prop removal is safe: no consumers outside packages/web-shell/. The sidebar.newWorktreeTask i18n key is correctly kept — it's still used as an aria-label for the fork icon badge on worktree sessions (line 2571 of WebShellSidebar.tsx).
• i18n.tsx — new keys (worktree.toggleHint, worktree.cancel) defined in both EN and ZH. The shortened worktree.welcomeDesc still conveys the key information.
• CSS — 154 new lines in App.module.css for the toggle/badge styling (entrance animation, hover glow, icon medallion). More visual polish than the minimal path, but appropriate for a user-facing UI feature.
• Tests — 6 new unit tests in App.test.tsx (visibility gating for trusted/untrusted/non-git, toggle on/off, session creation with/without worktree), 3 new Playwright E2E tests, and updated WorkspaceSection test. Coverage looks solid.
• mockDaemon.ts — adds GET /workspaces/:cwd/git route support for E2E tests. Clean addition.

Typecheck passes cleanly (tsc --noEmit).

Real-Scenario Testing

This is a browser UI change (web-shell), so the real-scenario evidence comes from Playwright E2E tests driving a real Chromium browser against the actual web-shell UI with a mock daemon:

Running 3 tests using 1 worker

  ✓  1 [chromium] › client/e2e/web-shell.worktree-toggle.spec.ts:51:1 › enabling the worktree toggle sends worktree intent on session creation (4.3s)
  ✓  2 [chromium] › client/e2e/web-shell.worktree-toggle.spec.ts:78:1 › cancelling the toggle omits worktree on session creation (2.0s)
  ✓  3 [chromium] › client/e2e/web-shell.worktree-toggle.spec.ts:102:1 › toggle is hidden when the workspace is not a git repository (1.0s)

  3 passed (9.1s)

Unit tests (vitest):

 App.test.tsx — 149 passed (149)  [includes 6 new worktree toggle tests]
 WorkspaceSection.test.tsx — 7 passed (7)

 Test Files  2 passed (2)
      Tests  156 passed (156)

Dev server starts and serves correctly:

  VITE v5.4.21  ready in 212 ms

  ➜  Local:   http://localhost:5199/
  ➔  Network: use --host to expose

Running 3 tests using 1 worker

  ✓  1 [chromium] › enabling the worktree toggle sends worktree intent on session creation (4.3s)
  ✓  2 [chromium] › cancelling the toggle omits worktree on session creation (2.0s)
  ✓  3 [chromium] › toggle is hidden when the workspace is not a git repository (1.0s)

  3 passed (9.1s)

 App.test.tsx — 149 通过 (149)  [含 6 个新 worktree toggle 测试]
 WorkspaceSection.test.tsx — 7 通过 (7)

 Test Files  2 通过 (2)
      Tests  156 通过 (156)

— Qwen Code · qwen3.7-max

_{Reviewed at e19f022f75f58904c14e24b856f45f412da294cf · re-run with @qwen-code /triage}

Confidence: 4/5 — clean, focused UI improvement with solid test coverage; only non-blocking nit is the elaborate CSS polish.

This PR does one thing well: it takes an existing feature that was effectively hidden (worktree isolation, buried in a sidebar dropdown that only rendered under several conditions) and puts it where users actually start a session — the empty state. The implementation is the simplest possible path: the toggle just sets the existing pendingWorktreeRef intent, reusing the entire pending-worktree state machine and lazy session creation. No SDK changes, no daemon changes, no new failure surface.

The sidebar simplification (dropdown → direct button) is a natural consequence of removing the worktree menu item — leaving a single-item dropdown would be worse UX than a direct click. The onNewWorktreeSession prop removal is clean with no external consumers.

Tests are comprehensive: 6 unit tests covering all gating conditions and the toggle lifecycle, 3 Playwright E2E tests driving a real browser, and the existing 156-test suite passes without regression. Typecheck is clean.

The CSS is more elaborate than strictly necessary (entrance animations, hover glow, icon medallion), but for a user-facing discoverability feature, visual polish is part of the deliverable — not scope creep.

Approving. ✅

— Qwen Code · qwen3.7-max

_{Reviewed at e19f022f75f58904c14e24b856f45f412da294cf · re-run with @qwen-code /triage}

@qwen-code /takeover

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on:

Autofix review response — PR #7365

Addressed both inline suggestions from the automated reviewer. No conflict
(--conflict false), so origin/main was not merged. One additive commit:
6a9186e57.

Feedback point 1 — "clickable" git chip test never clicks

File: packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx:138
Classification: Optional (**[Suggestion]**) — accepted.

The test name promised a "clickable" chip but only asserted DOM attributes
(tagName, data-dirty, className, aria-label); it never exercised the
onClick handler, so a deleted or miswired handler would pass silently.

Change: kept the existing assertions and added an interaction check in the
file's own style (manual createRoot + act + DOM event dispatch — this suite
does not use Testing Library). The chip is a read-only <output>; the test now
locates the wrapping <button> via chip.closest('button'), dispatches a
bubbling click, and asserts onOpenGitDiff was called with the workspace cwd
(/tmp/project). I did not copy the reviewer's snippet verbatim because it
assumed Testing Library APIs and a data-dirty="false" state that contradicts
this test's unstaged: 1 fixture.

Feedback point 2 — dead worktree option on the new-session path

File: packages/web-shell/client/components/sidebar/WebShellSidebar.tsx:3547
Classification: Optional (**[Suggestion]**) — accepted.

This PR removed the only caller that passed { worktree: {} } (the former
onNewWorktreeSession prop); the empty-state toggle now sets
pendingWorktreeRef directly. The worktree field on the new-session option
type was therefore dead and invited a future caller through an untested path.

Change: removed worktree from the option type on the onNewSession prop
signature and on handleNewSession in WebShellSidebar.tsx, and from
createNewSession in App.tsx (keeping opts for keepView). The sidebar
handler is now (workspaceCwd) => createNewSession(workspaceCwd).

Behavior is preserved exactly: createNewSession previously ran
pendingWorktreeRef.current = opts?.worktree, and since no caller passed
worktree, that always evaluated to undefined. Those two lines are now an
explicit pendingWorktreeRef.current = undefined; setWorktreePending(false);
(with a short comment explaining why a fresh chat drops the toggle's pending
intent), so a plain "new chat" still clears any pending worktree exactly as
before.

Verification

• npm run typecheck — pass
• npm run lint — pass
• npm run build — pass
• Focused Vitest (web-shell): WorkspaceSection.test.tsx 7/7 pass;
  App.test.tsx + WebShellSidebar.workspace-removal.test.tsx +
  WebShellSidebar.collapse-persist.test.tsx 180/180 pass

No settings source changed, so no settings-schema regeneration was required.

Base-conflict check: no conflict with main.

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human.

🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

Code review — surface worktree isolation in the new-session empty state

Overview

Moves the worktree-isolation entry point out of the sidebar git-pill dropdown and into the new-session empty state as a visible toggle. It reuses the existing pending-worktree state machine and lazy (first-prompt) session creation, so there are no SDK/daemon changes. The sidebar git pill is simplified from a dropdown to a single native button that opens the Changes view directly. Ships unit + Playwright E2E + visual-screenshot coverage, an i18n copy pass (en/zh), and a design doc.

I traced the core path and it holds up:

• Toggle → pendingWorktreeRef.current = {} / setWorktreePending(true) → first prompt → sendPrompt → ensureSessionForPrompt reads pendingWorktreeRef.current → creates the worktree session → finally clears the pending intent. The unit/E2E tests assert worktree: {} reaches the POST /session body. ✅
• Eligibility (trusted && gitStatus.branch) is resolved against activeWorkspaceCwd, which is the same workspace resolution ensureSessionForPrompt uses at submit time (locked ?? selected ?? primary), and selectedWorkspaceGitStatus is fetched keyed on that same cwd — so the gate and the creation target can't diverge. ✅
• welcomeHeader (and hence the toggle) is passed to MessageList only when isChatEmptyState, so the toggle can't leak into an active session even though the header row otherwise renders at the top of the transcript. ✅
• createNewSession now unconditionally clears the pending intent; every remaining caller passes at most { keepView }, so no caller relies on the removed worktree opt. No orphaned i18n keys (sidebar.newWorktreeTask is still used by the session-row fork badge; sidebar.worktreeDescription and its only usage are both removed). ✅

Strengths

• Genuinely low-risk: reuses the state machine and mirrors the old sidebar gating, so no new failure surface — matches the PR's risk statement.
• Good test coverage across layers: unit (visibility gates, on/off, submit body with/without worktree), E2E (worktree intent in the request body, cancel path, non-git hidden), and a new gotoNewSession visual helper that is the suite's only view of the empty state — nice catch to keep it from being invisible to the preview.
• Mock-daemon /workspaces/:cwd/git route defaults to a null-branch status, so it's additive: existing scenarios that don't set gitStatus keep hiding the chip exactly as before.

Findings (all minor — nothing blocking)

1. Focus management on the toggle↔badge swap (a11y). Clicking the toggle unmounts it and mounts the badge; focus falls back to <body>. Same on cancel (✕). Keyboard/AT users lose their place both directions. Consider moving focus to the cancel button on enable and back to the toggle on cancel. — App.tsx

2. Git-pill accessible name / tooltip (minor a11y regression). The new wrapper <button aria-label={t('gitDiff.title')}> gives the pill an explicit name ("Changes"), which overrides the inner <output aria-label="…branch main…"> — so the branch/dirty summary is dropped from the pill's accessible name (previously the role="button" span computed its name from that content). And because the tooltip trigger is the non-focusable <output>, the focusable element is now the outer button, so keyboard focus no longer surfaces the branch tooltip. Net it's still an upgrade (native button + single click beats span+dropdown), but consider folding the branch into the button's aria-label so the summary isn't lost. — WorkspaceSection.tsx

3. Test robustness — hardcoded promise ticks. waitForToggle() awaits the git-status fetch with three fixed flush() calls. If that promise chain ever gains a tick, the unit tests break silently. Prefer vi.waitFor(() => expect(container.querySelector(toggleSelector)).not.toBeNull()). — App.test.tsx

4. Test setup — good-git-repo default lives in afterEach, not beforeEach. The workspaceByCwd→{ branch: 'main' } mock is restored in afterEach, so each test depends on the previous test's teardown to start from a working git repo. That's order-fragile and unconventional; establishing the default in beforeEach makes each test self-contained. — App.test.tsx

Nits

• worktreeToggleEligible is recomputed unmemoized every render and feeds the welcomeHeader useMemo deps — fine in practice (cheap .find() returning a primitive), just noting.
• The off-state control is a plain <button> rather than role="switch" / aria-pressed. Acceptable since it's a one-way reveal rather than a checkbox, but a screen reader announces "button", not a toggle state.

Verdict: Clean, well-tested, low-risk discoverability win. Recommend addressing the two a11y items (1, 2) and the test-robustness items (3, 4) as follow-up polish, none of which block merge.

🤖 Could not address the latest feedback automatically (round 2/100). A human should take over this PR.

What I found before stopping:
Qwen failed during address-review: status 1.

See the Qwen Autofix agent step logs for model/tool output.

Run log: https://github.com/QwenLM/qwen-code/actions/runs/29791533733

🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

@qwen-code /retry

🔄 AutoFix re-armed. The next scan re-reads this PR's feedback from the start and the round counter resets. Nothing was deleted — this marker supersedes the evaluation markers above it.

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on:

Review feedback addressed

Inline comments

[rc:3618264941] WorkspaceSection.test.tsx — test should click the button and assert onOpenGitDiff
Already addressed in a prior commit on this branch. The test at line 163–168 already dispatches a click on the wrapping <button> and asserts onOpenGitDiff was called with /tmp/project. No new change needed.

[rc:3618264957] WebShellSidebar.tsx — dead worktree option on createNewSession / handleNewSession / onNewSession
Already addressed in a prior commit on this branch. The worktree option was removed from the onNewSession prop signature, handleNewSession, and the createNewSession opts type. The onNewWorktreeSession prop was also removed from WorkspaceSection. No new change needed.

[rc:3618626603] App.tsx — no test for sidebar "New chat" clearing pending worktree intent
✅ Implemented. Added test 'clears the pending worktree intent when starting a new session from the sidebar': enables the toggle, clicks the sidebar "New chat" button, submits a message, and verifies createSession is called without worktree.

Issue-level review (@wenshao)

1. Focus management on toggle↔badge swap (a11y)
✅ Implemented. Added worktreeToggleRef, worktreeCancelRef, and a worktreeFocusTarget ref. A useEffect on worktreePending moves focus to the cancel button on enable and back to the toggle on cancel, so keyboard/AT users don't lose their place.

2. Git-pill accessible name includes branch (a11y)
✅ Implemented. Changed the git-pill button's aria-label from t('gitDiff.title') (just "Changes") to `${t('gitDiff.title')} — ${gitStatus.branch}` (e.g. "Changes — main"), so the branch name is no longer dropped from the accessible name.

3. Test robustness — hardcoded promise ticks
✅ Implemented. Replaced the three fixed flush() calls in waitForToggle() with vi.waitFor(() => expect(...).not.toBeNull()), which retries until the assertion passes regardless of how many promise ticks the git-status fetch takes.

4. Test setup — mock default in afterEach → beforeEach
✅ Implemented. Moved the workspaceByCwd mock (returning { branch: 'main' }) from afterEach into beforeEach, so each test is self-contained and not order-dependent.

Nit: worktreeToggleEligible unmemoized
Declined — no functional impact; the reviewer noted it is "fine in practice" (cheap .find() returning a primitive). Memoizing would add a deps array for negligible gain.

Nit: role="switch" / aria-pressed on the toggle
Declined — the reviewer noted this is "Acceptable since it's a one-way reveal rather than a checkbox." Adding switch semantics would misrepresent the interaction model.

Verification

• npx tsc --noEmit (web-shell): ✅ pass
• npx eslint on all 3 changed files: ✅ pass
• npx vitest run client/App.test.tsx: ✅ 150/150 pass
• npx vitest run client/components/sidebar/WorkspaceSection.test.tsx: ✅ 7/7 pass

Base-conflict check: no conflict with main.

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human.

🧠 Handled by Qwen Code · model/模型 qwen3.8-max-preview

Code Review Summary

Verdict: Approved ✅

Statistics

• Effort: high
• Files: 17 (550 source, 480 test, 83 docs)
• Build: 5 workspaces passed
• Tests: 1230 passed (38 files, 0 failures)

Findings

• Critical: 0
• Suggestion: 1 (inline)
• Nice to have: 0

Suggestion

packages/web-shell/client/App.tsx:6010-6019 — Focus management useEffect lacks test coverage. If removed or misconfigured, keyboard users could lose focus to <body> after cancel (WCAG 2.4.3 regression). Non-blocking.

Summary

Well-structured feature PR that surfaces worktree isolation in the new-session empty state. Comprehensive test coverage across unit, E2E, and visual regression layers. Code quality is good with proper state management and cleanup paths.

— qwen3.7-max via Qwen Code /review

Released in v0.20.1.

The comment is just the release bot announcing that PR #7365 shipped in v0.20.1 — no action needed. Acknowledged, no code changes or reply required.

✅ completed